Faiza and Rosalyn by Faiza1987 · Pull Request #27 · Ada-C11/VideoStoreAPI

Faiza1987 · 2019-05-17T22:29:10Z

Video Store API

Congratulations! You're submitting your assignment!
If you didn't get to the functionality the question is asking about, reply with what you would have done if you had completed it.

Comprehension Questions

Question	Answer
Explain how you came up with the design of your ERD, based on the seed data.	Seed data showed movies and customers, it doesn’t tell you much about the relationship but from that you could assume that a customer could potentially have many movies, which each movie could have many customers who rent it.Rentals therefore is what really joins together the movie and the customer. So rental would include both the movie_is and customer_id.
What would be the Big-O time complexity of your /customers & /movies endpoints? What does the time complexity depend on? Explain your reasoning.	Linear O(n) with n being the number of customers and movies. It depends on the number of customers in the system as well as the number of movies you have available for rental. More customers or movies increases the amount of time it will take to return the information for the action you need.
What is the Big-O time complexity of the `POST /rentals/check-in` endpoint? What does the time complexity depend on? Explain your reasoning.	Currently it is Constant O(1). When a checkin is performed, the action is performed on 1 movie only. However, it is very possible that in the future the rental contains multiple movies that have been checked out stored as an array. In that case it would be Linear O(n) with n being the number of movies that needs to be checked in.
Describe a set of positive and negative test cases you implemented for a model.	set_checkin_date is a method in rental model. Positive test sets check in date to current and currently checked out too false. In a negative test case, if the movie is already checked out then it returns false.
Describe a set of positive and negative test cases you implemented for a controller.	Our movies create method. Positive test case it that it successfully creates a new movie given valid data. Our movie count increases by 1, we render json which is a hash and includes an ID. When we look for that movie using the id, the movie will have the same title as the one created. In a negative test case in which we try to create a movie without a title, we render json which should be a hash, it must include errors along with a response of bad request.
How does your API respond when bad data is sent to it?	It renders a json as false with a status of bad request as well as the error code associated with that request.
Describe one of your custom model methods and why you chose to wrap that functionality into a method.	Set_check_out date was one of the custom methods we created. It made more sense to have the logic that goes into setting date in the model as it is daily complex
Do you have any recommendations on how we could improve this project for the next cohort?	Wave 3 and optional requirements blend into each other which can be a little confusing
Link to Trello	Did not use a trello board in this case. As there was only 2 of us and we were in close proximity we could divide up the work as we went.
Link to ERD	https://drive.google.com/file/d/17Th57MrorEM5yO5sa6ustUyUVWhiOuft/view?usp=sharing
Summary

exit

… tests

droberts-sea · 2019-05-23T17:24:34Z

Video Store

What We're Looking For

Feature	Feedback
Core Requirements
Git hygiene	yes
Comprehension questions	yes
General
Business Logic in Models	yes
All required endpoints return expected JSON data	yes
Requests respond with appropriate HTTP Status Code	yes
Errors are reported	yes
Testing
Passes all Smoke Tests	yes
Model Tests - all relations, validations, and custom functions test positive & negative cases	yes
Controller Tests - URI parameters and data in the request body have positive & negative cases	yes
Overall	Great work overall!

droberts-sea · 2019-05-23T17:14:01Z

app/controllers/movies_controller.rb

+    def checkout
+        rental = Rental.new(rental_params)
+
+        rental.set_checkout_date


I like the idea of making set_checkout_date be its own method. If you were to take this even further, you could write a class method on Rental that does both in one step.

droberts-sea · 2019-05-23T17:17:00Z

app/controllers/movies_controller.rb

+
+    def checkin
+        rental = Rental.find_by(movie_id: rental_params[:movie_id], customer_id: rental_params[:customer_id])
+


I don't see that using strong params here gets you anything. It would be simpler to say:

Rental.find_by(movie_id: params[:movie_id], customer_id: params[:customer_id])

Since we're not using params directly to create or update a model (as in Rental.new(params)), we don't need the filtering that strong params gives us.

droberts-sea · 2019-05-23T17:18:06Z

app/models/customer.rb

+  def movies_checked_out_count
+    currently_rented = Rental.where(customer_id: self.id, currently_checked_out: true)
+    return currently_rented.length
+  end


Because you have the relation on line 2, you could shorten line 14 to rentals.where(currently_checked_out: true).

I do love that you calculate this value on the fly instead of storing it in the database. That means there's no way to forget to update the count.

droberts-sea · 2019-05-23T17:19:03Z

app/models/movie.rb

+  def available_inventory
+    checked_out_movies = Rental.where(movie_id: self.id, currently_checked_out: true)
+    avail_inventory = inventory - checked_out_movies.length
+    return avail_inventory


Similar comments to Customer#movies_checked_out_count

droberts-sea · 2019-05-23T17:21:21Z

test/controllers/customers_controller_test.rb

+
+    it "returns json" do
+      # Arrange - Act
+      get customers_path


The code you've written here matches what we showed in class. However, those tests were intentionally split out as a pedagogical tool. I think you could get away with collapsing all of these into one, since there's only one behavior you're covering.

droberts-sea · 2019-05-23T17:23:31Z

test/controllers/movies_controller_test.rb

+
+      get movie_path(invalid_movie_id)
+
+      must_respond_with :bad_request


Since your endpoint returns specific JSON in this case, it would be wise to verify that is sent correctly.

Faiza1987 and others added 30 commits May 14, 2019 13:40

Initial commit with models, controllers, and seeded database

89dafd7

exit

movie model testing complete

4fcc919

Added model tests for customers

2c674a2

Resolved conflict

2f45b65

Model controller tests complete

74ef880

Added index action in customers controller, added customer controller…

fcf6d08

… tests

merged conflicts

cf74cf0

Fixed movie_params to pass Postman smoke tests

7b95e61

Added more tests for Movies controller

5b5a616

rental controller generated

45b8419

merge conflict fixed

a242866

wrote checkin method

73cace6

Merge branch 'wave3_checkin'

bc0a0ae

Added a checkout method for rentals

a51948d

commit message

1a51ad5

Dropped join table and modified relationships between models

5f53b9b

Created a new migration for rentals

8d23394

Trying to modify checkout and checkin

fb5efde

Modified checkin and checkout

4ff3075

Added composition relationship and validation in the rentals model

ee2b243

Added CustomerId and MovieId as foreign keys to Rentals

d072268

Added model methods to rental and movie models

cf99ef1

Fixed checkout method

91c70ee

Added a checkin_date column for rentals tables

a0613d8

Added methods for checkin and finished checkin method

51fc6a0

Fixed checkin method. Smoke tests are all passing

87f09c0

Added checkout method test in movies controller

60a853f

test for checkin passing

1402527

Merging

e681a1a

Added tests for rental model

37802bf

ranuseh and others added 3 commits May 17, 2019 12:47

Added tests for rental model

15a725b

modified movie controller tests

8ccb5c1

Added more tests

7b3a650

droberts-sea reviewed May 23, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Faiza and Rosalyn#27

Faiza and Rosalyn#27
Faiza1987 wants to merge 33 commits intoAda-C11:masterfrom
Faiza1987:master

Faiza1987 commented May 17, 2019 •

edited

Loading

Uh oh!

droberts-sea commented May 23, 2019

Uh oh!

droberts-sea May 23, 2019

Uh oh!

droberts-sea May 23, 2019

Uh oh!

droberts-sea May 23, 2019

Uh oh!

droberts-sea May 23, 2019

Uh oh!

droberts-sea May 23, 2019

Uh oh!

droberts-sea May 23, 2019

Uh oh!

droberts-sea May 23, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		def checkin
		rental = Rental.find_by(movie_id: rental_params[:movie_id], customer_id: rental_params[:customer_id])


		get movie_path(invalid_movie_id)

		must_respond_with :bad_request

Conversation

Faiza1987 commented May 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Video Store API

Comprehension Questions

Uh oh!

droberts-sea commented May 23, 2019

Video Store

What We're Looking For

Uh oh!

droberts-sea May 23, 2019

Choose a reason for hiding this comment

Uh oh!

droberts-sea May 23, 2019

Choose a reason for hiding this comment

Uh oh!

droberts-sea May 23, 2019

Choose a reason for hiding this comment

Uh oh!

droberts-sea May 23, 2019

Choose a reason for hiding this comment

Uh oh!

droberts-sea May 23, 2019

Choose a reason for hiding this comment

Uh oh!

droberts-sea May 23, 2019

Choose a reason for hiding this comment

Uh oh!

droberts-sea May 23, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Faiza1987 commented May 17, 2019 •

edited

Loading